-
Notifications
You must be signed in to change notification settings - Fork 360
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor Nushell script #1763
Refactor Nushell script #1763
Conversation
7489ac2
to
6c90f5c
Compare
Closes nushell#12148. Will simplify jdx/mise#1763. Request for discussion.
Closes nushell#12148. Will simplify jdx/mise#1763. Request for discussion.
Closes nushell#12148. Will simplify jdx/mise#1763. Request for discussion.
Rewrite of nushell/nushell#12156 for jdx/mise#1763. ### Why? Nushell philosophically omits a `set` list mutation. But `$env` is inherently mutable leading to issues described in nushell/nushell#12148. `set-env` provides such an operation exclusively for `$env`. ### What changed? 1. Explicit flag instead of implicit list concatenation 2. Expands updates to any `$env` field not only `$env.config` ### How is it used? ```yaml ❯ set-env -h Gracefully set an environment variable or merge a nested option. Examples: Set $env.NUPM_HOME > set-env NUPM_HOME $'($nu.home-path)/.local/share/nupm' Add to $env.NU_LIB_DIRS > set-env --append NU_LIB_DIRS $'($env.NUPM_HOME)/modules' Set a nested config option > set-env config.filesize.metric true Add a config hook > set-env -a config.hooks.pre_prompt 'ellie | print' Usage: > main {flags} <field> <value> Flags: -a, --append - Append to the previous value or wrap in a new list -h, --help - Display the help message for this command Parameters: field <cell-path>: The environment variable name or nested option cell path value <any>: The value to set or append Input/output types: ╭───┬─────────┬─────────╮ │ # │ input │ output │ ├───┼─────────┼─────────┤ │ 0 │ nothing │ nothing │ ╰───┴─────────┴─────────╯ ``` ### How does it work? ```nushell export def --env main [ field: cell-path value: any --append (-a) ]: nothing -> nothing { # just an alias def 'get or' [default field] { get --ignore-errors $field | default $default } let value = if $append { # append to the previous value or empty list $env | get or [] $field | append $value } else { $value } # work around nushell/nushell#12168 let field = $field | to text | split row . let value = match $field { [_] => $value # if cell path is nested [$root, ..$field] => { let field = $field | into cell-path # reassigning $env would be an error # merging reserved names like PWD would be an error # so merge from 1 level deep instead $env | get or {} $root | upsert $field $value } } # avoid issues noted above load-env { ($field | first): $value } } ``` ### Where are the tests? Pending next PR for nupm integration.
Nice improvement! |
thanks for the contribution, and sorry for the delay, I have a lot on my plate right now. That said, I think this generally looks good, I had some clarifying questions and there is a test failure probably related to snapshots that need to be updated. |
No worries I just didn't want to leave the impression this was a drive-by PR!
My apology this deserved more documentation! Writing it up outside review comments so it doesn't get lost for future maintenance. Nu originally used
TLDR:
Files are modules and commands named TLDR:
I also wanted to add types to this |
thanks for the explanation! |
yeah I think that's quite a bit beyond "refactor" |
2eaafe3
to
e47d857
Compare
Requested change complete and added related issue to the description
Would you be open to such a change? Extract other scripts too or only Nu? Happy to continue on Discord 💬 |
possibly... though it may conflict with some of the work I'm doing on usage so I'm not sure right now |
Then I'll start with a minimal draft and continue if it plays nice with your usage ✌🏼 |
Depends on jdx/mise#1763 in release.
Depends on jdx/mise#1763 in release.
Depends on jdx/mise#1763 in release.
Depends on jdx/mise#1763 in release.
PS ready to merge I believe and linked a docs PR when it's released |
you've got a test failure |
Rename Nushell param based on core team feedback
e47d857
to
bade126
Compare
I think I fixed the snapshot but CI says |
* Update Nushell integration Depends on jdx/mise#1763 in release. * Automate reinstallation Manually picked from starship/starship#5845.
Rewrite of nushell/nushell#12156 for jdx/mise#1763. ### Why? Nushell philosophically omits a `set` list mutation. But `$env` is inherently mutable leading to issues described in nushell/nushell#12148. `set-env` provides such an operation exclusively for `$env`. ### What changed? 1. Explicit flag instead of implicit list concatenation 2. Expands updates to any `$env` field not only `$env.config` ### How is it used? ```yaml ❯ set-env -h Gracefully set an environment variable or merge a nested option. Examples: Set $env.NUPM_HOME > set-env NUPM_HOME $'($nu.home-path)/.local/share/nupm' Add to $env.NU_LIB_DIRS > set-env --append NU_LIB_DIRS $'($env.NUPM_HOME)/modules' Set a nested config option > set-env config.filesize.metric true Add a config hook > set-env -a config.hooks.pre_prompt 'ellie | print' Usage: > main {flags} <field> <value> Flags: -a, --append - Append to the previous value or wrap in a new list -h, --help - Display the help message for this command Parameters: field <cell-path>: The environment variable name or nested option cell path value <any>: The value to set or append Input/output types: ╭───┬─────────┬─────────╮ │ # │ input │ output │ ├───┼─────────┼─────────┤ │ 0 │ nothing │ nothing │ ╰───┴─────────┴─────────╯ ``` ### How does it work? ```nushell export def --env main [ field: cell-path value: any --append (-a) ]: nothing -> nothing { # just an alias def 'get or' [default field] { get --ignore-errors $field | default $default } let value = if $append { # append to the previous value or empty list $env | get or [] $field | append $value } else { $value } # work around nushell/nushell#12168 let field = $field | to text | split row . let value = match $field { [_] => $value # if cell path is nested [$root, ..$field] => { let field = $field | into cell-path # reassigning $env would be an error # merging reserved names like PWD would be an error # so merge from 1 level deep instead $env | get or {} $root | upsert $field $value } } # avoid issues noted above load-env { ($field | first): $value } } ``` ### Where are the tests? Pending next PR for nupm integration.
Rewrite of nushell/nushell#12156 for jdx/mise#1763. ### Why? Nushell philosophically omits a `set` list mutation. But `$env` is inherently mutable leading to issues described in nushell/nushell#12148. `set-env` provides such an operation exclusively for `$env`. ### What changed? 1. Explicit flag instead of implicit list concatenation 2. Expands updates to any `$env` field not only `$env.config` ### How is it used? ```yaml ❯ set-env -h Gracefully set an environment variable or merge a nested option. Examples: Set $env.NUPM_HOME > set-env NUPM_HOME $'($nu.home-path)/.local/share/nupm' Add to $env.NU_LIB_DIRS > set-env --append NU_LIB_DIRS $'($env.NUPM_HOME)/modules' Set a nested config option > set-env config.filesize.metric true Add a config hook > set-env -a config.hooks.pre_prompt 'ellie | print' Usage: > main {flags} <field> <value> Flags: -a, --append - Append to the previous value or wrap in a new list -h, --help - Display the help message for this command Parameters: field <cell-path>: The environment variable name or nested option cell path value <any>: The value to set or append Input/output types: ╭───┬─────────┬─────────╮ │ # │ input │ output │ ├───┼─────────┼─────────┤ │ 0 │ nothing │ nothing │ ╰───┴─────────┴─────────╯ ``` ### How does it work? ```nushell export def --env main [ field: cell-path value: any --append (-a) ]: nothing -> nothing { # just an alias def 'get or' [default field] { get --ignore-errors $field | default $default } let value = if $append { # append to the previous value or empty list $env | get or [] $field | append $value } else { $value } # work around nushell/nushell#12168 let field = $field | to text | split row . let value = match $field { [_] => $value # if cell path is nested [$root, ..$field] => { let field = $field | into cell-path # reassigning $env would be an error # merging reserved names like PWD would be an error # so merge from 1 level deep instead $env | get or {} $root | upsert $field $value } } # avoid issues noted above load-env { ($field | first): $value } } ``` ### Where are the tests? Pending next PR for nupm integration.
Closes #589 and fixes #589 (comment).
Resolves the following issues:
source
will eventually be deprecatedexport def main
makesmise
the default import fromuse mise.nu
mise.nu
mistakenly overrides user$env.config.hooks.display_output
(others too)set-env
nushell/nu_scripts#787 for built-in solution to$env
collisionsadd-hook
workaround explained belowPATH
configuration more resilient in case it's a liststd path add
API (still undocumented Generate documentation forstd
commands nushell/nushell.github.io#1210)It would facilitate maintenance if the script could be extracted with
include_bytes!
and use vanillastr::replace
instead offormatdoc!
to avoid escaping braces.